feat: add bottom_margin config for prompt positioning#115
feat: add bottom_margin config for prompt positioning#115brancengregory wants to merge 2 commits intoeitsupi:mainfrom
Conversation
|
Thank you for your contribution! I did some research and it seems that if we implement such a function independently in arf, it may cause inconsistencies with the internal state of reedline. |
|
Seems reasonable to me! I'll take a look. Thanks for your work on this, huge fan! |
|
Thanks! It might be a good idea to add an argument to |
|
So I tried implementing the bottom margin in reedline here per your suggestion about I'm not seeing another approach beyond the current one that maintains reedline's state consistency while handling external output. Do you see something I'm missing? |
|
Sidenote, I think [prompt]
bottom_margin = { type = "proportional", value = 0.5 }or [prompt]
bottom_margin = { type = "fixed", value = 10 } |
|
Thanks for trying the reedline approach and sharing the results! I looked into Painter internals and I agree That said, I have some feedback on the current implementation:
|
- adds BottomMargin enum with Fixed, Proportional, and Disabled variants
- places config in [editor] section for terminal positioning control
- implements ensure_bottom_margin() with single ScrollUp call
- early returns for disabled states: Fixed(0) and Proportional(>=1.0)
- clamps Fixed to terminal height and Proportional to 0.0-1.0 range
- derives Default trait idiomatically with #[default] attribute
- adds 4 tests for parsing Fixed, Proportional, Disabled, and default
- updates JSON schema and snapshots
TOML configuration:
bottom_margin = { fixed = 10 } # Reserve 10 lines at bottom
bottom_margin = { proportional = 0.5 } # Reserve bottom 50%
bottom_margin = "disabled" # No margin (default)
- test proportional margin keeps prompt in upper half (0.5) - test fixed margin reserves exact line count (5 lines) - test disabled margin allows prompt at bottom - test proportional = 0.0 pins prompt to top - test fixed = 0 behaves like disabled (zero-cost) - test large fixed values clamp to terminal height - test high proportional values (0.95) keep prompt near top - test window resize handling and margin consistency
|
Okay I made those changes, cleaned up the config format a bit, and attempted to do some PTY tests. Also updated the PR title and initial description comment |
|
The only thing I didn't address is this point:
That is still the case but I think this is an inherent tradeoff if we don't want the prompt line to drift at all. If some drift were tolerable then I guess some kind of 'every X iteration' would be possible but I don't think that would be the ideal UX. I personally think the 2-5ms per prompt is acceptable as long as it stays an opt-in feature. |
eitsupi
left a comment
There was a problem hiding this comment.
Thank you. I'll take a look when I have time.
For now, I'm curious about the format of the configuration file.
| "enum": [ | ||
| "disabled" | ||
| ] |
There was a problem hiding this comment.
In the world of TOML, where there are no nulls, I think not setting anything represents null.
I think that instead of setting a special value "disabled" here, the default should be to not display the line.
Implements #114